Skip to content

Conversation

MackinnonBuck
Copy link

@MackinnonBuck MackinnonBuck commented Aug 21, 2025

Summary

Makes the following changes:

  • Removes the McpEndpoint abstract class
  • Replaces interfaces with abstract classes:
    • IMcpEndpoint -> McpSession
    • IMcpServer -> McpServer
    • IMcpClient -> McpClient
  • Renames internal types:
    • McpClient -> McpClientImpl
    • McpServer -> McpServerImpl
    • McpSession -> McpSessionHandler
  • Removes factory classes:
    • McpClientFactory
      • Factory method moved directly into the McpClient abstract class
    • McpServerFactory
      • Factory method moved directly into the McpServer abstract class
  • Updates McpClientImpl and McpServerImpl to function without relying on a common base class implementing shared functionality

Note

See the edit history of this comment to view previous revisions

Description

This change generally aligns with the suggestions in this comment.

An alternative could be to remove McpClient factory methods altogether and do something similar to what was suggested here. However, this means that we're differentiating between a "client" and a "client session", and the MCP specification doesn't make such a distinction. In the future, we could still add new factory methods that make McpClient creation more concise. For example:

using var client = await McpClient.ConnectAsync("http://localhost:3001/sse");

Or:

using var client = await McpClient.LaunchAsync("npx", "-y", "@modelcontextprotocol/server-everything");

Fixes #355

@PederHP
Copy link
Collaborator

PederHP commented Aug 21, 2025

One downside to this renaming is that it distances the SDK from the TypeScript and Python SDKs, which makes it harder for developer to transition directly between. It's a very minor drift - adding postfixing -Session to McpClient. But I still think it's worth considering every time drift is introduced whether it is strictly necessary or a unified naming can be kept.

It is also preferable, I think, if McpClient aligns directly with the Client concept as described in: https://modelcontextprotocol.io/specification/2025-06-18/architecture.

I was never happy with McpClientFactory so I am glad to see it go, but definitely don't think it should be renamed to McpClient, as that was cause conceptual confusion with the Client concept in the specification. A client is a client session within a host.

Restructuring is definitely warranted, but I'd like to caution against drift from the other SDKs. There's a lot of polyglot development going in this space, and there are many upsides to having at least the specification concepts named identically in the SDKs.

With this change the SDK uses McpClient to mean something other than the specification and the other SDKs and uses McpClientSession to refer to the concept of a client. I can see the point of using the latter, but the former I am skeptical of. A client is a very well defined concept in the protocol spec and in the other SDKs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert IMcpEndpoint, IMcpClient, and IMcpServer types to classes.
2 participants